feat: Local CAS now uses IPFS-compatible CIDs #411

DRK3 · 2021-05-19T17:07:19Z

The local CAS now uses CIDs that are compatible with IPFS, assuming that the IPFS node is running under default settings and that the data is less than 256KB.

A new flag was added to choose between v1 or v0 CIDs. The IPFS client will also use this format now. v1 CIDs are the default.

Signed-off-by: Derek Trider [email protected]

closes #318

codecov · 2021-05-19T19:28:41Z

Codecov Report

Merging #411 (3ec5760) into main (4fd4d92) will decrease coverage by 0.12%.
The diff coverage is 85.29%.

@@            Coverage Diff             @@
##             main     #411      +/-   ##
==========================================
- Coverage   89.13%   89.00%   -0.13%     
==========================================
  Files          93       93              
  Lines        6184     6202      +18     
==========================================
+ Hits         5512     5520       +8     
- Misses        392      400       +8     
- Partials      280      282       +2

Impacted Files	Coverage Δ
cmd/orb-server/startcmd/start.go	`82.75% <66.66%> (ø)`
cmd/orb-server/startcmd/params.go	`80.45% <80.00%> (-0.02%)`	⬇️
pkg/store/cas/cas.go	`92.30% <87.50%> (+1.83%)`	⬆️
pkg/cas/ipfs/ipfs.go	`80.00% <100.00%> (+3.52%)`	⬆️
...pub/service/inbox/httpsubscriber/httpsubscriber.go	`88.73% <0.00%> (-11.27%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fd4d92...3ec5760. Read the comment docs.

sandrask · 2021-05-19T21:21:31Z

Now that CIDs are the same as IPFS can you enable announce test? Look for: """ TODO: Enable this test when CIDs for ipfs and local CAS are identical

DRK3 · 2021-05-19T22:57:18Z

@sandrask I enabled it and it worked. However...

Per an earlier discussion with @troyronda, for this commit I'm not going to be actually changing over the format yet. The reason is to make sure that I'm not changing the sample CIDs that we're submitting to the test suite before having proper default v1 support.

So while I have tested locally with my new compatible v0 CIDs enabled, for this PR I need to leave it disabled. I'll re-enable it in #344

troyronda · 2021-05-19T23:22:12Z

pkg/store/cas/cas.go

+		cid = merkledag.NodeWithData(unixfs.FilePBData(content, uint64(len(content)))).Cid().String()
+	} else {
+		prefix := gocid.Prefix{
+			Version:  0,


Why version 0

That's what it was set to before.

That's what our current IPFS client does. I need to update it in another PR to start using v1 as well...

(changing it to v1 will break the current sample CIDs)

The current sample CIDs seem
incorrect, being neither an accepted v0 nor a v1.

What are/ where are the current sample CIDs? Maybe they were produced using the current (faulty) v0 generation in Orb?

In a previous PR I updated the CID in a sample create activity in our BDD tests, but that was to match the CID output by IPFS (by default), so it should be valid. https://github.com/trustbloc/orb/pull/339/files#r626128523

DRK3 · 2021-05-20T19:51:20Z

test/bdd/features/activitypub.feature

@@ -112,7 +112,6 @@ Feature:
    Then the JSON path "type" of the response equals "CollectionPage"
    And the JSON path "items" of the response does not contain "${domain2IRI}"

-""" TODO: Enable this test when CIDs for ipfs and local CAS are identical


@sandrask Actually, per a follow-up discussion with @troyronda it looks like we need to make the CIDs we produce compatible with IPFS now. v1 CIDs are now the default and they work with IPFS. I re-enabled the tests again and they pass.

sandrask · 2021-05-20T20:06:24Z

cmd/orb-server/startcmd/params.go

@@ -629,6 +649,7 @@ func createFlags(startCmd *cobra.Command) {
 	startCmd.Flags().StringP(httpSignaturesEnabledFlagName, httpSignaturesEnabledShorthand, "", httpSignaturesEnabledUsage)
 	startCmd.Flags().StringP(casTypeFlagName, casTypeFlagShorthand, "", casTypeFlagUsage)
 	startCmd.Flags().StringP(ipfsURLFlagName, ipfsURLFlagShorthand, "", ipfsURLFlagUsage)
+	startCmd.Flags().String(cidVersionFlagName, "0", cidVersionFlagUsage)


Does this mean to default to "0" if not provided?

Oops, yes you're correct. That should be "1". I've fixed it now.

sandrask · 2021-05-20T20:10:49Z

pkg/cas/ipfs/ipfs_test.go

@@ -74,7 +74,7 @@ func TestRead(t *testing.T) {
 		}))
 		defer ipfs.Close()

-		cas := New(ipfs.URL)
+		cas := New(ipfs.URL, false)


missing test with true

Done. I also noticed that the "success" test was using a mocked server. Since I already went to the work of using a real IPFS server in the cas_test.go tests, I updated it to use a real IPFS server as well.

sandrask · 2021-05-20T20:14:21Z

pkg/store/cas/cas.go

+	if p.useV0CIDs {
+		// The v0 CID produced below is equal to what an IPFS node would produce, assuming that:
+		// 1. The IPFS node is running with default settings, and
+		// 2. The size of the content passed in here is less than 256KB (the default chunk size).


Is there a plan to make it work with content > 256KB? If there is a plan maybe we should create an issue to track this.

Created an issue: #418

The local CAS now uses CIDs that are compatible with IPFS, assuming that the IPFS node is running under default settings and that the data is less than 256KB. A new flag was added to choose between v1 or v0 CIDs. The IPFS client will also use this format now. v1 CIDs are the default. Signed-off-by: Derek Trider <[email protected]>

cla-bot bot added the cla-signed label May 19, 2021

DRK3 force-pushed the IPFSCompatibleCIDs branch from b46f625 to abe4630 Compare May 19, 2021 19:26

DRK3 force-pushed the IPFSCompatibleCIDs branch from abe4630 to 1879506 Compare May 19, 2021 21:02

DRK3 marked this pull request as ready for review May 19, 2021 21:02

DRK3 force-pushed the IPFSCompatibleCIDs branch 2 times, most recently from d0ab325 to c408f5c Compare May 19, 2021 21:12

DRK3 changed the title ~~feat: Use IPFS-compatible CIDs in CAS~~ feat: Local CAS support for writing IPFS-compatible v0 CIDs May 19, 2021

DRK3 force-pushed the IPFSCompatibleCIDs branch from c408f5c to 99362af Compare May 19, 2021 22:52

DRK3 force-pushed the IPFSCompatibleCIDs branch from 99362af to bd7cbc8 Compare May 19, 2021 23:04

troyronda reviewed May 19, 2021

View reviewed changes

fqutishat approved these changes May 20, 2021

View reviewed changes

DRK3 changed the title ~~feat: Local CAS support for writing IPFS-compatible v0 CIDs~~ WIP: feat: Local CAS support for writing IPFS-compatible v0 CIDs May 20, 2021

DRK3 force-pushed the IPFSCompatibleCIDs branch from bd7cbc8 to 2e1df0f Compare May 20, 2021 19:48

DRK3 changed the title ~~WIP: feat: Local CAS support for writing IPFS-compatible v0 CIDs~~ WIP: feat: Local CAS now uses IPFS-compatible CIDs May 20, 2021

DRK3 force-pushed the IPFSCompatibleCIDs branch from 2e1df0f to 63abdce Compare May 20, 2021 19:50

DRK3 commented May 20, 2021

View reviewed changes

DRK3 force-pushed the IPFSCompatibleCIDs branch from 63abdce to d3b8ae8 Compare May 20, 2021 19:58

DRK3 changed the title ~~WIP: feat: Local CAS now uses IPFS-compatible CIDs~~ feat: Local CAS now uses IPFS-compatible CIDs May 20, 2021

DRK3 requested a review from troyronda May 20, 2021 20:02

sandrask reviewed May 20, 2021

View reviewed changes

DRK3 force-pushed the IPFSCompatibleCIDs branch 4 times, most recently from 8ee9c32 to 87e920a Compare May 20, 2021 20:47

DRK3 requested a review from sandrask May 20, 2021 20:48

sandrask approved these changes May 20, 2021

View reviewed changes

DRK3 force-pushed the IPFSCompatibleCIDs branch from 87e920a to 3ec5760 Compare May 20, 2021 21:06

bstasyszyn approved these changes May 20, 2021

View reviewed changes

fqutishat merged commit 8e15c53 into trustbloc:main May 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Local CAS now uses IPFS-compatible CIDs #411

feat: Local CAS now uses IPFS-compatible CIDs #411

DRK3 commented May 19, 2021 •

edited

Loading

codecov bot commented May 19, 2021 •

edited

Loading

sandrask commented May 19, 2021 •

edited

Loading

DRK3 commented May 19, 2021

troyronda May 19, 2021

DRK3 May 20, 2021

DRK3 May 20, 2021

troyronda May 20, 2021

DRK3 May 20, 2021

DRK3 May 20, 2021

sandrask May 20, 2021

DRK3 May 20, 2021

sandrask May 20, 2021

DRK3 May 20, 2021

sandrask May 20, 2021 •

edited

Loading

DRK3 May 20, 2021

feat: Local CAS now uses IPFS-compatible CIDs #411

feat: Local CAS now uses IPFS-compatible CIDs #411

Conversation

DRK3 commented May 19, 2021 • edited Loading

codecov bot commented May 19, 2021 • edited Loading

Codecov Report

sandrask commented May 19, 2021 • edited Loading

DRK3 commented May 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandrask May 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DRK3 commented May 19, 2021 •

edited

Loading

codecov bot commented May 19, 2021 •

edited

Loading

sandrask commented May 19, 2021 •

edited

Loading

sandrask May 20, 2021 •

edited

Loading